Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for creating foreign key constraints using the references type in the migration bake command. The feature allows developers to specify foreign key relationships directly in the command line when generating migrations, automatically handling the column creation, foreign key constraint setup, and table name inference.
Key changes:
- New
parseForeignKeys()method to extract and process foreign key definitions from command arguments - Template updates to render foreign key constraints in generated migration files
- Enhanced argument parsing to handle the
referencestype and convert it to integer columns with foreign key constraints
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Util/ColumnParser.php | Adds foreign key parsing logic with automatic table name inference from field names |
| src/Command/BakeMigrationCommand.php | Integrates foreign key parsing and updates help documentation with examples |
| templates/bake/config/skeleton.twig | Adds template logic to render foreign key constraints in generated migrations |
| tests/TestCase/Util/ColumnParserTest.php | Adds comprehensive test coverage for foreign key parsing functionality |
| tests/TestCase/Command/BakeMigrationCommandTest.php | Adds integration tests and teardown cleanup for foreign key migration tests |
| tests/comparisons/Migration/*.php | Expected output files for migration generation tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Command/BakeMigrationCommand.php
Outdated
| * The <warning>index</warning> attribute can define the column as having a unique | ||
| key with <warning>unique</warning> or a primary key with <warning>primary</warning>. | ||
| * Use <warning>references</warning> type to create a foreign key constraint. | ||
| e.x. <warning>category_id:references</warning> (auto-infers table as 'categories') |
There was a problem hiding this comment.
Corrected abbreviation from 'e.x.' to 'e.g.' (exempli gratia - for example).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| $files = glob(ROOT . DS . 'config' . DS . 'Migrations' . DS . '*_*Posts.php'); | ||
| if ($files) { | ||
| foreach ($files as $file) { | ||
| unlink($file); | ||
| } | ||
| } | ||
|
|
||
| $files = glob(ROOT . DS . 'config' . DS . 'Migrations' . DS . '*_*Articles.php'); | ||
| if ($files) { | ||
| foreach ($files as $file) { | ||
| unlink($file); | ||
| } | ||
| } | ||
|
|
||
| $files = glob(ROOT . DS . 'config' . DS . 'Migrations' . DS . '*_*Products.php'); | ||
| if ($files) { | ||
| foreach ($files as $file) { | ||
| unlink($file); | ||
| } | ||
| } |
There was a problem hiding this comment.
The teardown cleanup logic is duplicated for three different file patterns. Consider extracting this into a helper method that accepts a pattern parameter to reduce code duplication.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@dereuromark I've opened a new pull request, #946, to work on those changes. Once the pull request is ready, I'll request review from you. |
* Initial plan * Fix abbreviation: change 'e.x.' to 'e.g.' in help text Co-authored-by: dereuromark <39854+dereuromark@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: dereuromark <39854+dereuromark@users.noreply.github.com>
Resolves #318
I put it into 4.x as it should be pretty safe, given it adds only specific parsing on top.
So this can already be used before we go to the next major. Especially since we are going Cake5.3+.
This would probably be the last minor release on that 4.x branch then.
Let me know if we should target 5.x right away, though.